Allow decoding while debugging Yul sources in Solidity 0.8.21 (and related changes) #6154
Conversation
|
Blech, #6157 is a substantial caveat on this PR it turns out... I still think this should be merged, because it's ridiculous for us to not get the AST, but it does mean things are rarely going to work properly in this case... :-/ |
|
I don't think we should present incorrect values. As a compromise, what if we get the AST, but don't use it until #6157 is resolved? |
|
I mean, I'm not sure there's really anything we can do to fix #6157. Moreover, Solidity is fixing the bug that made us have to do extra work to get the Yul AST, so once that's fixed and released, it won't be a question of whether we'll be getting the Yul AST, because we will, without having to do extra work! I guess the only question really is whether we deliberately ignore Yul ASTs, or something... |
|
OK, Nick thinks this is fine to merge as-is, so I'm going to do that! But I will go bug the Solidity team like Amal says... |
This PR allows decoding of variables while decoding Yul sources that were compiled with solc 0.8.21 or greater. Note this PR does not address #3831, I figured that was better saved for later.
It was already possible to decode Yul variables in Solidity
assemblyblocks, but not in straight Yul files, because Solidity wouldn't give us ASTs for the latter. But now it does!Well, now it does if we ask for them right. For some reason, Solidity makes us ask for the ast at the contract level when compiling Yul, instead of at the file level like normal; I've filed an issue about that (although for 0.8.21 compatibility we'll have to leave this extra bit in anyway). Also, there's another bug regarding these (which I've also filed an issue about) -- it assigns the singular source an ID of 1 instead of 0. So, I had to add in a workaround for that as well.
Once we have the ASTs, the debugger can do the rest, so not many more changes were needed! But there are two.
First off, it turns out that the top-level node in a Yul AST, the
YulObjectnode type, doesn't have ansrcfield. So the code in Codec that gets the source index from the top node of an AST needed some adjustment in that case. Not only does theYulObjectnode lack this info, but so does theYulCodenode below it; one has to go to theYulBlocknode atast.code.blockto get it. Fortunately these nodes are always present -- I tried. (Of course we could also just always return 0, but hey, if the info is there let's use it, right? :P )Second off, since an AST might now have
YulObjectrather thanSourceUnitas its top-level node, a bunch of code in data that assumed thatSourceUnitis the only type of top-level node has been updated to account forYulObjectas well. Some of this got factored out intohelpers.Finally, I added a test! One caveat -- the test did not work right once I introduced mutation. O_o I assume this is due to something with the optimizer or something? I decided to just omit that from the test, which is not great, but I didn't see what else I could reasonably do.
Note that adding this test obviously involved increasing the Solidity version used in the debugger tests to 0.8.21, and for some reason that slowed down a few tests, so I increased the timeout on those few.
I also tested this manually using the
yul-testproject in the solidity-test-cases repo.